Skip to content

fix(enterprise): escape pipes and newlines in Markdown Actions table (#338) - #355

Closed
cnYui wants to merge 1 commit into
FreshCode-Org:mainfrom
cnYui:fix/md-table-cell-escaping
Closed

cnYui wants to merge 1 commit into
FreshCode-Org:mainfrom
cnYui:fix/md-table-cell-escaping

Conversation

@cnYui

@cnYui cnYui commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Problem

In the Actions table of QualityReport.to_markdown(), only the description
cell was escaped. A column named a|b therefore added an extra cell and
shifted the whole row, and a newline in any cell split the row. This breaks the
rendered table (issue #338).

Reproduction on main:

import pandas as pd, freshdata as fd
from freshdata.enterprise import build_quality_report

df = pd.DataFrame({"a|b": [" x", "y ", "z", "w"], "k": [1, 2, 3, 4]})
out, rep = fd.clean(df, return_report=True, verbose=False, column_names=False)
print(build_quality_report(df, out, rep).to_markdown())
# Actions data row has 5 cells while the header has 4.

Fix

  • Escape every cell inside _md_table_row via a small _md_escape_cell
    helper: | becomes \|, and \r\n / \r / \n become <br>.
  • Remove the now-redundant a.description.replace("|", "\|") at the call site
    so values are no longer double-escaped.

The separator/header rows contain no pipes or newlines, so their rendering is
unchanged.

Tests

Verification (run locally)

  • pytest -m "not online and not large" — coverage gate reached (93.21%); the
    only failures are 3 pre-existing, unrelated tests (performance/test_runner_cli,
    test_experimental_ai_copilot, test_semantic_backends) that also fail on an
    unmodified main in this environment.
  • pytest tests/test_enterprise_metrics.py --no-cov — all pass.
  • ruff check . — clean.

I also added a note under Unreleased → Fixed in CHANGELOG.md.

QualityReport.to_markdown() escaped only the description cell, so a column
name containing "|" added an extra cell and shifted the Actions row, and
newlines in any cell split the row. Escape every cell inside _md_table_row
instead: "|" becomes "\|" and line breaks become "<br>". The manual
per-description escape is removed so values are no longer double-escaped.

Fixes FreshCode-Org#338
@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: a578cdf3-3ae8-4a81-85df-a0bbe305736f


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kevincostner17

Copy link
Copy Markdown
Contributor

Thanks! The fix looks good. I ran tests/test_enterprise_metrics.py and ruff locally on Python 3.9 and 3.12, and all CI checks pass.

One small thing before it can merge: #350 was just merged and also added a bullet at the top of CHANGELOG.md → Unreleased → Fixed, so this branch now conflicts there. It's the only conflict. Could you rebase on main and keep both bullets (the on_low_score one from #350 and yours)? I'll merge once it's green.

@kevincostner17

Copy link
Copy Markdown
Contributor

Thank you @cnYui for this fix! Your branch needed a rebase onto current main to resolve a CHANGELOG.md conflict, so I've opened #404, which carries your original commit unchanged with your authorship preserved. Once #404 merges, this PR will be closed as superseded, and the fix for #338 will land under your name.

@kevincostner17

Copy link
Copy Markdown
Contributor

Superseded by #404, which merged with @cnYui's original commit and authorship preserved. #338 is fixed on main. Thank you again for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants